-
Notifications
You must be signed in to change notification settings - Fork 3.2k
improvement(copilot): added session context checks in copilot tool calls #1466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR strengthens security by adding session context checks to copilot tool calls, ensuring users are authenticated before tools execute on their behalf.
Key improvements:
- Added
verifyWorkflowAccess()function with comprehensive permission checking for workflow ownership and workspace access - Modified all user-related server tools (
get-environment-variables,set-environment-variables,get-oauth-credentials) to require authenticated user context - Enhanced the tool execution pipeline to pass
userIdfrom authenticated sessions through to individual tools - Implemented proper error handling with descriptive permission error messages
- Replaced direct
getUserId()calls with authenticated context verification
Security enhancements:
- Tools now verify user authentication before accessing sensitive data like environment variables and OAuth credentials
- Workflow access is properly validated against database ownership and workspace permissions
- Prevents unauthorized tool execution by requiring active authenticated sessions
Confidence Score: 5/5
- This PR is safe to merge with no security risks
- The implementation follows security best practices with proper authentication, authorization checks, comprehensive error handling, and no breaking changes to existing functionality
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/lib/copilot/auth/permissions.ts | 5/5 | Added comprehensive workflow access verification with proper permission checks and error handling |
| apps/sim/app/api/copilot/execute-copilot-server-tool/route.ts | 5/5 | Enhanced to pass authenticated userId context to tool execution for security |
| apps/sim/lib/copilot/tools/server/user/get-environment-variables.ts | 5/5 | Added authentication checks and workflow access verification before accessing environment variables |
| apps/sim/lib/copilot/tools/server/user/set-environment-variables.ts | 5/5 | Implemented authentication and workflow access controls before allowing environment variable modifications |
| apps/sim/lib/copilot/tools/server/user/get-oauth-credentials.ts | 5/5 | Added session context validation and workflow access verification for OAuth credential access |
| apps/sim/lib/copilot/tools/server/base-tool.ts | 5/5 | Updated interface to include optional userId context parameter for authentication |
| apps/sim/lib/copilot/tools/server/router.ts | 5/5 | Enhanced to accept and forward userId context to individual tools for authentication |
Sequence Diagram
sequenceDiagram
participant C as Copilot Client
participant A as API Endpoint
participant R as Tool Router
participant T as Server Tool
participant P as Permissions
participant D as Database
C->>A: Execute tool request
A->>A: Authenticate session
alt Authentication fails
A-->>C: Unauthorized error
else Authentication succeeds
A->>R: Route execution with user context
R->>T: Execute tool with context
alt Workflow access required
T->>P: Verify workflow access
P->>D: Check permissions
D-->>P: Return access data
P-->>T: Return access result
alt Access denied
T-->>A: Permission error
A-->>C: Forbidden response
else Access granted
T->>T: Process tool execution
T-->>R: Return result
R-->>A: Forward result
A-->>C: Success response
end
else No workflow check needed
T->>T: Process tool execution
T-->>R: Return result
R-->>A: Forward result
A-->>C: Success response
end
end
18 files reviewed, no comments
ecbb0ff to
eee8c91
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
added session context checks in copilot tool calls for added security, to ensure user's have an authenticated context before the copilot executes tools on their behalf
Type of Change
Testing
Manually.
Checklist